q: Do I understand it correctly, that http.client.request.error has been removed, because http.client.response.finish was here first, so http.client.request.error is kinda useless in our case, as finish did the trick already?!

If so, I wonder what would happen on newer Node versions where http.client.response.finish is not getting triggered - I think I'm missing something

Ok, I think you're actually noticing a valid point here. (It's very confusing!)

So, the flow is:

• create ClientRequest object
• fire http.client.request.created (node >=22.12 only)
• pass object to user
• user does things, calls .end() at some point.
• Headers sent!
• fire http.client.request.start (all node versions)
• response arrives from server
• user consumes response
• fire http.client.response.finish (all node versions)

("All node versions" means >=18.6, so, all that we support anyway.)

If at some point, the request has an error, it'll fire http.client.request.error, but only on node >=22.6.0 || 20 >=20.17.0.

On node 22 >=22.12.0 || 23 >=23.2.0 || >=24 we have the http.client.request.created, and we attach an errorMonitor to the request object, so there's no need for http.client.request.error on those versions.

On nodes that don't support http.client.request.created, if breadcrumbs are enabled, we generate breadcrumbs on the http.client.response.finish. (This doesn't matter if we attached to request.created, because we do the errorMonitor and prependListener('response') listeners early enough.)

However, the valid point is that the response is not guaranteed, if the request has a socket hangup error or something before the response arrives.

I think the solution is that for old nodes, we attach a listener on the http.client.request.start diagnostics channel. This gets access to the request object, albeit too late to set headers, so no trace propagation. But it can attach the error listener to do breadcrumbs without a response. This covers more node versions than the http.client.request.error channel, and we don't need to have another version range to be checking.

On newer node versions, we're relying on the fact that the request.created channel listener attaches a response event handler, and then we deal with the response at that point.

l: There is already the _INTERNAL_getTracingHeadersForFetchRequest which is quite similar, with the difference that it doesn't apply the headers, but returns them. Theoretically we can try to reuse it, but not really mandatory for this PR.

l: Yeah good idea, it might make sense there, as these utilities there are quite similar. I would then suggest to move mergeBaggage there and maybe call it mergeBaggageHeader?!

q/l: Would that mean we can remove the @opentelemetry/instrumentation-http dependency as well?

Ah, yes, I had thought we needed that for the Server bits for now, but in fact we do not. Good catch!

q/m: Do you know what will happen for users that already have an OTel setup? Usually the HTTP instrumentation is on by default when doing auto instrumentation. Before it wouldn't do double instrumentation, as we already registered it, but now we removed this and OTel can now instrument it.

This is a good point! Also, it's going to affect any instrumentations that we migrate from OTel to Sentry for use in other platforms, right?

This does add to the argument in favor of changing the span origin from auto.http.otel.http to auto.http.client (similarly for the express instrumentation). So, if users do start getting duplicate instrumentations, they'll know why.

I'll try to set up a demo app with this scenario, and see if there's a way we can detect the situation or somehow register with OTel in that case.

this is a good point! IMHO it is not a blocker if we can't get this to work. but maybe we can find a way to at least warn if we detect this somehow… but this may be hard due to timing, as we may be the first or last to wrap. ideal world, if possible:

1. warn if we wrap and detect that otel has already wrapped it
2. mark our wrapped stuff in a way that otel will also detect it as wrapped (??? not sure if this is feasible and/or has other problems attached to it…)

Also, it's going to affect any instrumentations that we migrate from OTel to Sentry for use in other platforms, right?

I think so yes.

This does add to the argument in favor of changing the span origin from auto.http.otel.http to auto.http.client (similarly for the express instrumentation). So, if users do start getting duplicate instrumentations, they'll know why.

Fair point. Maybe we can even set up a document somewhere where we basically have potential double instrumentation and would recommend to not do autoinstrumentation but allow list these instrumentations. Some "best practice" guide or something.

mark our wrapped stuff in a way that otel will also detect it as wrapped

Could work potentially - timing is an issue as you mentioned.

let's mention here somewhere that this has to be called before the request is started, for future reference?

Trace propagation guard for existing sentry-trace removed

High Severity

The new injectTracePropagationHeaders checks for existing headers individually. When a sentry-trace header is already present, it still injects Sentry's traceparent and merges Sentry's DSC into baggage. This creates inconsistent trace propagation headers, which can break distributed tracing downstream.

 

^{Reviewed by Cursor Bugbot for commit 5c11ce7. Configure here.}

This is exactly the logic that was previously in packages/node-core/src/utils/outgoingHttpRequest.ts.

mergeBaggage reverses non-Sentry entry precedence

Medium Severity

The new mergeBaggage iterates the incoming baggage last and unconditionally writes every key, so non-Sentry entries from incoming overwrite identically-keyed non-Sentry entries already present on the request. The replaced mergeBaggageHeaders helper documented and implemented the opposite rule: non-Sentry entries from the existing header took precedence, and only Sentry-prefixed entries were overwritten. Outgoing baggage values that an application or middleware set on a request can now be silently replaced by current-scope values.

 

^{Reviewed by Cursor Bugbot for commit 5c11ce7. Configure here.}

Light http integration loses trace propagation on Node <22

Medium Severity

The node-core light httpIntegration only injects sentry-trace / baggage via the http.client.request.created diagnostic channel, which does not exist before Node 22.12. The fallback added for older Nodes (http.client.request.start) only attaches breadcrumb listeners — it cannot inject headers because that channel fires after headers have been sent. The corresponding integration test was un-gated from conditionalTest({ min: 22 }) and now unconditionally asserts sentry-trace and baggage are set on /api/v0 and /api/v1, which will fail when the suite runs against Node <22.

 

^{Reviewed by Cursor Bugbot for commit 5c11ce7. Configure here.}

Suppressed tracing also disables breadcrumbs unconditionally

Low Severity

onHttpClientRequestCreated returns immediately when SUPPRESS_TRACING_KEY is set, so when a user wraps a call in Sentry.suppressTracing(...) no breadcrumb is recorded and no propagation is attempted — even when breadcrumbs: true and propagateTrace: false. Suppressing tracing semantically should suppress spans/propagation, not breadcrumbs. The previous diagnostics-channel paths for breadcrumbs (http.client.response.finish and http.client.request.error) were not gated on this key.

 

^{Reviewed by Cursor Bugbot for commit 9abf370. Configure here.}

Outgoing request URL no longer normalized

Medium Severity

getRequestUrl builds the URL by simple string concatenation, whereas the previous getBreadcrumbData and similar helpers passed the constructed URL through new URL(...), which normalized things like default-port stripping and path resolution. As a result, breadcrumb url, http.url, and tracePropagationTargets matching can now see values like http://example.com:80/ where they previously saw http://example.com/, and any user-configured tracePropagationTargets regex that assumed the normalized form may no longer match. This can also flow through to getSanitizedUrlString(parseUrl(url)), where unparsed concatenation is more likely to fail.

 

^{Reviewed by Cursor Bugbot for commit 9abf370. Configure here.}

Breadcrumb timing changes on older Node versions

Low Severity

The fallback path for Node versions without http.client.request.created now subscribes to http.client.request.start and emits the outgoing request breadcrumb synchronously inside the 'response' listener — i.e. when the response starts, not when it ends. The old light-mode code used http.client.response.finish, which fires after the response body fully arrives. As a result, on older Nodes the breadcrumb's relative ordering versus subsequent events shifts, and the breadcrumb is recorded before the response is actually complete.

 

^{Reviewed by Cursor Bugbot for commit 9abf370. Configure here.}

Patched http.get drops options object handling

Medium Severity

The replacement patchedGet calls httpModule.request.apply(this, args) and then request.end(). Node's real http.get performs additional argument normalization (e.g. forwarding the URL/options/callback triple correctly and handling agent/Agent defaults) before delegating to request. Although in current Node get is essentially request(...) ; req.end(), replacing it wholesale (rather than wrapping the original) discards any future or platform-specific behavior baked into the original get, including handling of options.agent === false semantics that some custom HTTP modules implement.

 

^{Reviewed by Cursor Bugbot for commit 9abf370. Configure here.}

Node's http.get just does:

function get(url, options, cb) {
  const req = request(url, options, cb);
  req.end();
  return req;
}

So there really isn't any argument normalization or anything.

Response error breadcrumb omits response in non-span path

Low Severity

In the no-spans branch, the response errorMonitor listener calls addOutgoingRequestBreadcrumb(request, undefined), dropping the available response object. This means breadcrumbs for partially-received responses lose their status_code and log level even though the response was reached. The corresponding span-enabled branch passes response to the breadcrumb in finishWithResponse, so the two paths emit breadcrumbs with different fidelity for the same scenario.

 

^{Reviewed by Cursor Bugbot for commit ef6e736. Configure here.}

Bug: An early return in onHttpClientRequestCreated causes a ReferenceError when accessing addedBreadcrumbs in async event listeners because it's declared too late.
_{Severity: CRITICAL}

Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/core/src/integrations/http/client-subscriptions.ts#L70-L76

Potential issue: In the `onHttpClientRequestCreated` function, when `createSpans` is
false, event listeners are registered that will later call the `addBreadcrumbs`
function. However, an early `return` is executed before the `addedBreadcrumbs` variable
is declared and initialized. Due to JavaScript's Temporal Dead Zone (TDZ) for `let`
declarations, when the asynchronous event listeners fire, the call to `addBreadcrumbs`
attempts to access the uninitialized `addedBreadcrumbs` variable. This results in a
`ReferenceError`, which will crash any outgoing HTTP request made when using the light
integration or when spans are otherwise disabled.

Also affects:

• packages/core/src/integrations/http/client-subscriptions.ts:110~116

TDZ ReferenceError in no-spans breadcrumb path

High Severity

When createSpans is false, the addedBreadcrumbs variable is not initialized because its declaration is after an early return. This causes a ReferenceError when event listeners call addBreadcrumbs, preventing outgoing request breadcrumbs from being added when spans are disabled (e.g., light mode or tracing off).

 

^{Reviewed by Cursor Bugbot for commit 645dc0e. Configure here.}